countries- project by chen kaduri #22
Conversation
| @@ -29,7 +29,7 @@ | |||
| </head> | |||
| <body> | |||
| <!-- Loader --> | |||
| <div class="loader"> | |||
| <div class="loader" id="loader"> | |||
| <div class="spinner"> | |||
There was a problem hiding this comment.
Why do you need both class and id ?
|
|
||
| if (!countryName) { | ||
| document.getElementById('country-name').innerHTML = '<p>No country selected!</p>'; | ||
| return; |
There was a problem hiding this comment.
Do not use innerHTML , it is a dangerous method
| } | ||
|
|
||
| fetch('CountriesData.json') | ||
| .then(response => { |
There was a problem hiding this comment.
Hardcoded strings should be saved into constants file
|
|
||
| //If the country is found, display its details | ||
| if (country) { | ||
| // Create the main container |
There was a problem hiding this comment.
Please avoid explaining the code with comments, is a bad practice.
The code should explain itself
|
|
||
| // Create the flag section | ||
| const flagContainer = document.createElement('div'); | ||
| flagContainer.className = 'country-flag'; |
There was a problem hiding this comment.
If you need to explain this with comment you can extract this into function named generateFlagSection and just call it instead of the comment
There was a problem hiding this comment.
This comment apply to everywhere in your code
| const countryTitle = document.createElement('h1'); | ||
| countryTitle.textContent = country.name; | ||
| infoContainer.appendChild(countryTitle); | ||
|
|
There was a problem hiding this comment.
This is good instead of using innerHTML
| countryRegion.appendChild(strongR); | ||
| countryRegion.appendChild(document.createTextNode(country.region)); | ||
| countryInfo.appendChild(countryRegion); | ||
|
|
There was a problem hiding this comment.
In this page you have a lot of similar code again an again, can you think about a way to avoid it?
doing the project about countrie with use in css, js, html, json.
added filter function fetch the data from json.